Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GNMI TypedValue to scalar value conversion and update supported platforms #7173

Merged
merged 2 commits into from
Mar 17, 2020

Conversation

sbyx
Copy link
Contributor

@sbyx sbyx commented Mar 13, 2020

This uses the GNMI library native ToScalar function to convert GNMI TypedValues to scalar Go values.
Also adds an updated note on tested device platforms.

Fixes #7167

@@ -324,23 +323,9 @@ func (c *CiscoTelemetryGNMI) handleTelemetryField(update *gnmi.Update, tags map[
return aliasPath, nil
}

value, _ := gnmivalue.ToScalar(update.Val)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the error and also there are some value types we should probably skip, such as []byte. The field needs to be one of int64, float64, uint64, string, bool, there is some massaging that is done when acc.AddField is called but that was done mostly to avoid errors with older plugins, we try do it in the plugin now.

Is this going to work with the JsonVal/JsonIetfVal types? Maybe we should do the ToScaler as the default case of the switch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your points, I therefore reverted those changes and just changed the handling for the decimalval type. I think in hindsight it would be more trouble switching to the native casting function since we would then need to deal with the error cases as it also does not deal with the Json types which we would need to handle manually anyway.

@danielnelson danielnelson added this to the 1.14.0 milestone Mar 16, 2020
@danielnelson danielnelson merged commit 064247a into influxdata:master Mar 17, 2020
denzilribeiro pushed a commit to denzilribeiro/telegraf that referenced this pull request Mar 27, 2020
athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
veorlo pushed a commit to GlobalNOC/telegraf that referenced this pull request May 4, 2020
HarshitOnGitHub pushed a commit to HarshitOnGitHub/telegraf that referenced this pull request May 8, 2020
idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handling of Decimal64 for gnmi
2 participants